Do not choose constructors with an "Obsolete" attribute #224
Conversation
|
What's the performance impact? Why is this feature important? Why can't the code be changed - why is there an obsolete constructor at all? Is the benefit worth the performance impact? |
|
If there is one I haven't noticed it, and the performance tests pass (I realize there aren't any). Because using anything marked obsolete without explicitly requesting it (which can still be done by using [Inject]) is incorrect behavior in any context, especially from the perspective of an IoC container where it may not be obvious it is choosing the wrong thing. I can only give a specific answer from the perspective of my code, where legacy code that can not yet use Ninject has no choice but to instantiate objects the wrong way, but in general the obsolete attribute is used when deprecating functions that are still in use by legacy code. I haven't seen a performance impact (although I assume there is an imperceptible difference), so it comes down to whether choosing to use obsolete functions is considered correct. |
|
Fixed in 969b20b |
This heavily discourages Ninject from choosing constructors that are marked obsolete. I'm willing to bet that when Ninject chooses an obsolete constructor it was an unintentional mistake, and can go unnoticed.
I've replicated our specific mistake in the unit test, but there are multiple other scenarios. In our scenario an implicit binding can be created and used, but in the general case we'd rather have Ninject fail instead of using the obsolete constructor.